-
-
Notifications
You must be signed in to change notification settings - Fork 46
Loader prefix system for Modrinth #571
New issue
Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.
By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.
Already on GitHub? Sign in to your account
Conversation
You might have found this already https://docker-minecraft-server.readthedocs.io/en/latest/misc/contributing/development/#using-development-copy-of-tools but I have to admit it's not easiest testing workflow to get working initially. Your testing approach with direct command invocation is a lot of what I do anyway during development of mc-image-helper changes. |
itzg
left a comment
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Thanks. A couple minor tweaks and it seems good to me.
Co-authored-by: Geoff Bourne <[email protected]>
Co-authored-by: Geoff Bourne <[email protected]>
|
Alright that seems like it works for datapacks now |
This reverts commit c2672fb.
|
Even though I like most of the whitespace changes, I'm going to revert some of them since it's more important to have well bounded changes per PR. Please wait to push anything until I can push this next commit. |
|
Pushed the changes. |
itzg
left a comment
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Ready for me to merge? I bet you are 😉
|
Yeah looks like that worked, was it line ending characters? If so I'm not sure why that happened because it seemed to be working with the previous commits |
|
Whitespace changes were most likely just one of the tools I used deciding it should reformat everything on saving. Usually I'd notice when the diff looks bigger than I expect but gets easier to miss with bigger changes. |
Wasn't quite sure in the end since git simply did the right thing as I pushed my changes. Yeah quite odd that it was only the one commit that did that. |
Been having trouble getting docker to include the file to locally test this, it builds and passes all tests but almost certainly not done in the best way. I was able to test it outside of the docker image with
./mc-image-helper modrinth --loader forge --projects "paper:simple-voice-chat, fabric:lithium, ferrite-core:beta, datapack:terralith, quilt:comforts:6.3.4+1.20.1" --game-version 1.20.1
It downloaded all successfully
Don't think this is ready to merge without heavy cleanup and testing but could use a second opinion on it
Closes #564